data/selinux, tests/main/selinux-clean: fine tune the policy, make sure that no denials are raised #6661
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add a spread test to monitor if basic snap management raises any SELinux denials. Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
…issues Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
Snap logs will trigger snapd to run journalct. On a SELinux system, this will cause a domain transition to journalctl_t. However, the policy does not allow journalctl to poke /proc data of pid 1, thus causing denials to appear in the log. Until this is fixed, all we can do is have a TODO note as a reminder. Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
- allow snap to exec snap-seccomp (for deriving system-key) - allow snap to manage directories/links/files under ~/snap - tweak snapd permissions to add remove links under /usr/share/bash-completion/completions (which is of usr_t type) - tweak permissions of snap-confine (can do a great deal with tmp_t, but reads were not enabled) Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
…ectory
Add a second socket to the snap, but this time it should be created in a
subdirectory. This should help uncover any problems with SELinux policy
preventing systemd from creating subdirectories under $SNAP_{DATA,COMMON}.
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
…nal-policy-update-clean-test
- allow managing char files under /var/snap, those could have been created by
snaps such as lxd
- allow fowner capabilty for snapd (snapshots?)
- allow snap-{update,discard}-ns to read tmpfs symlinks, etc. /etc/os-release
which is from /etc bind mounted on top of tmpfs
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
- allow querying SELinux enforcement mode - use proper interfaces to allow service reload/start/stop/enable/disable - account for cloud-init instance data access - account for /proc/sys/fs/may_detach_mounts probe in sanity - allow unliking incorrectly labeled socket files under /var/snap - account for polkit support poking /proc/<pid>/stat of the calling process Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
- allow snap-confine to create directories in ~/snap - allow dac_override for snap, when running as root Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
…nal-policy-update-clean-test
stolowski
reviewed
Apr 2, 2019
| @@ -156,6 +159,10 @@ systemd_config_all_services(snappy_t) | |||
| systemd_manage_all_unit_files(snappy_t) | |||
| systemd_manage_all_unit_lnk_files(snappy_t) | |||
| systemd_exec_systemctl(snappy_t) | |||
| systemd_reload_all_services(snappy_t) | |||
There was a problem hiding this comment.
Should we also have systemd_start_all_services?
There was a problem hiding this comment.
From inspecting the policy, this is likely covered by systemd_config_all_services(). This is what sesearch tells me:
$ sesearch -A -c service -s snappy_t
Found 2 semantic av rules:
allow snappy_t init_script_file_type : service { start stop status reload enable disable kill load } ;
allow snappy_t systemd_unit_file_type : service { start stop status reload enable disable kill load } ;
| #shellcheck source=tests/lib/snaps.sh | ||
| . "$TESTSLIB/snaps.sh" | ||
|
|
||
| install_local test-snapd-service |
There was a problem hiding this comment.
Can we also test snap start ... and snap stop ... here for this snap, or are these covered already elsewhere?
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
|
@Conan-Kudo can you take a look? |
stolowski
approved these changes
Apr 3, 2019
Conan-Kudo
approved these changes
Apr 3, 2019
|
@zyga do you think you could take a look at this PR too? |
zyga
reviewed
Apr 4, 2019
tests/main/selinux-clean/task.yaml
Outdated
|
|
||
| restore: | | ||
| rm -f stamp | ||
| setenforce 0 |
There was a problem hiding this comment.
Nitpick: I'd be cleaner if this test had restored the state of selinux rather than disabling it and assuming the initial state was the same.
Instead of assuming the mode should be permissive, restore to whatever was set before the test. Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
…nal-policy-update-clean-test
…nal-policy-update-clean-test
Services are started with their WorkingDirectory set to $SNAP_DATA. Since
snap-confine performs more checks on cwd now, we need to account for that in the
policy.
Relevant SELinux denial:
type=AVC msg=audit(1554975937.636:129): avc: denied { getattr } for pid=1099
comm="snap-confine" path="/var/snap/test-snapd-service/x1" dev="vda1"
ino=393657
scontext=system_u:system_r:snappy_confine_t:s0
tcontext=system_u:object_r:snappy_var_t:s0 tclass=dir permissive=1
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Final touches to the policy. Patch describes why those were needed: 261f16f
Add a test that performs basic snap operations with snaps that are typically run by the user (thus unconfined_t) and snaps that provide services, thus execute the whole targeted policy chain.